-
Notifications
You must be signed in to change notification settings - Fork 15.7k
[ADT] Make use of subsetOf and anyCommon methods of BitVector (NFC) #170876
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ADT] Make use of subsetOf and anyCommon methods of BitVector (NFC) #170876
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
@llvm/pr-subscribers-bolt Author: Anatoly Trosinenko (atrosinenko) ChangesReplace the code along these lines and with Full diff: https://github.com/llvm/llvm-project/pull/170876.diff 8 Files Affected:
diff --git a/bolt/include/bolt/Passes/LivenessAnalysis.h b/bolt/include/bolt/Passes/LivenessAnalysis.h
index 1df1113318d0b..f4faa1dc34ecd 100644
--- a/bolt/include/bolt/Passes/LivenessAnalysis.h
+++ b/bolt/include/bolt/Passes/LivenessAnalysis.h
@@ -37,10 +37,9 @@ class LivenessAnalysis : public DataflowAnalysis<LivenessAnalysis, BitVector,
virtual ~LivenessAnalysis();
bool isAlive(ProgramPoint PP, MCPhysReg Reg) const {
- BitVector BV = (*this->getStateAt(PP));
+ const BitVector &BV = *this->getStateAt(PP);
const BitVector &RegAliases = BC.MIB->getAliases(Reg);
- BV &= RegAliases;
- return BV.any();
+ return BV.anyCommon(RegAliases);
}
void run() { Parent::run(); }
diff --git a/bolt/include/bolt/Passes/ReachingDefOrUse.h b/bolt/include/bolt/Passes/ReachingDefOrUse.h
index 585d673e3b84e..41a6091aad4cb 100644
--- a/bolt/include/bolt/Passes/ReachingDefOrUse.h
+++ b/bolt/include/bolt/Passes/ReachingDefOrUse.h
@@ -133,8 +133,7 @@ class ReachingDefOrUse
RA.getInstClobberList(Point, Regs);
else
RA.getInstUsedRegsList(Point, Regs, false);
- Regs &= this->BC.MIB->getAliases(*TrackingReg);
- if (Regs.any())
+ if (Regs.anyCommon(this->BC.MIB->getAliases(*TrackingReg)))
Next.set(this->ExprToIdx[&Point]);
}
}
diff --git a/bolt/lib/Passes/RegReAssign.cpp b/bolt/lib/Passes/RegReAssign.cpp
index 0859cd244ce40..54eff51bfee68 100644
--- a/bolt/lib/Passes/RegReAssign.cpp
+++ b/bolt/lib/Passes/RegReAssign.cpp
@@ -316,18 +316,14 @@ void RegReAssign::aggressivePassOverFunction(BinaryFunction &Function) {
break;
}
- BitVector AnyAliasAlive = AliveAtStart;
- AnyAliasAlive &= BC.MIB->getAliases(ClassicReg);
- if (AnyAliasAlive.any()) {
+ if (AliveAtStart.anyCommon(BC.MIB->getAliases(ClassicReg))) {
LLVM_DEBUG(dbgs() << " Bailed on " << BC.MRI->getName(ClassicReg)
<< " with " << BC.MRI->getName(ExtReg)
<< " because classic reg is alive\n");
--End;
continue;
}
- AnyAliasAlive = AliveAtStart;
- AnyAliasAlive &= BC.MIB->getAliases(ExtReg);
- if (AnyAliasAlive.any()) {
+ if (AliveAtStart.anyCommon(BC.MIB->getAliases(ExtReg))) {
LLVM_DEBUG(dbgs() << " Bailed on " << BC.MRI->getName(ClassicReg)
<< " with " << BC.MRI->getName(ExtReg)
<< " because extended reg is alive\n");
diff --git a/bolt/lib/Passes/ShrinkWrapping.cpp b/bolt/lib/Passes/ShrinkWrapping.cpp
index fe342ccd38a67..b882e2512866d 100644
--- a/bolt/lib/Passes/ShrinkWrapping.cpp
+++ b/bolt/lib/Passes/ShrinkWrapping.cpp
@@ -1100,9 +1100,8 @@ SmallVector<ProgramPoint, 4> ShrinkWrapping::fixPopsPlacements(
bool Found = false;
if (SPT.getStateAt(ProgramPoint::getLastPointAt(*BB))->first ==
SaveOffset) {
- BitVector BV = *RI.getStateAt(ProgramPoint::getLastPointAt(*BB));
- BV &= UsesByReg[CSR];
- if (!BV.any()) {
+ const BitVector &BV = *RI.getStateAt(ProgramPoint::getLastPointAt(*BB));
+ if (!BV.anyCommon(UsesByReg[CSR])) {
Found = true;
PP = BB;
continue;
@@ -1110,9 +1109,8 @@ SmallVector<ProgramPoint, 4> ShrinkWrapping::fixPopsPlacements(
}
for (MCInst &Inst : llvm::reverse(*BB)) {
if (SPT.getStateBefore(Inst)->first == SaveOffset) {
- BitVector BV = *RI.getStateAt(Inst);
- BV &= UsesByReg[CSR];
- if (!BV.any()) {
+ const BitVector &BV = *RI.getStateAt(Inst);
+ if (!BV.anyCommon(UsesByReg[CSR])) {
Found = true;
PP = &Inst;
break;
diff --git a/bolt/lib/Passes/StackAvailableExpressions.cpp b/bolt/lib/Passes/StackAvailableExpressions.cpp
index a0d361f273de2..c685cc19badc3 100644
--- a/bolt/lib/Passes/StackAvailableExpressions.cpp
+++ b/bolt/lib/Passes/StackAvailableExpressions.cpp
@@ -103,8 +103,7 @@ bool StackAvailableExpressions::doesXKillsY(const MCInst *X, const MCInst *Y) {
else
RA.getInstClobberList(*Y, YClobbers);
- XClobbers &= YClobbers;
- return XClobbers.any();
+ return XClobbers.anyCommon(YClobbers);
}
BitVector StackAvailableExpressions::computeNext(const MCInst &Point,
diff --git a/bolt/lib/Passes/TailDuplication.cpp b/bolt/lib/Passes/TailDuplication.cpp
index 354f9b78830c3..e3ecf94a85070 100644
--- a/bolt/lib/Passes/TailDuplication.cpp
+++ b/bolt/lib/Passes/TailDuplication.cpp
@@ -97,8 +97,8 @@ bool TailDuplication::regIsPossiblyOverwritten(const MCInst &Inst, unsigned Reg,
getCallerSavedRegs(Inst, WrittenRegs, BC);
if (BC.MIB->isRep(Inst))
BC.MIB->getRepRegs(WrittenRegs);
- WrittenRegs &= BC.MIB->getAliases(Reg, false);
- return WrittenRegs.any();
+ const BitVector &AllAliases = BC.MIB->getAliases(Reg, false);
+ return WrittenRegs.anyCommon(AllAliases);
}
bool TailDuplication::regIsDefinitelyOverwritten(const MCInst &Inst,
@@ -117,8 +117,8 @@ bool TailDuplication::regIsUsed(const MCInst &Inst, unsigned Reg,
BinaryContext &BC) const {
BitVector SrcRegs = BitVector(BC.MRI->getNumRegs(), false);
BC.MIB->getSrcRegs(Inst, SrcRegs);
- SrcRegs &= BC.MIB->getAliases(Reg, true);
- return SrcRegs.any();
+ const BitVector &SmallerAliases = BC.MIB->getAliases(Reg, true);
+ return SrcRegs.anyCommon(SmallerAliases);
}
bool TailDuplication::isOverwrittenBeforeUsed(BinaryBasicBlock &StartBB,
diff --git a/llvm/lib/CodeGen/RDFRegisters.cpp b/llvm/lib/CodeGen/RDFRegisters.cpp
index 80fad722f592a..ee3e531c6fd5a 100644
--- a/llvm/lib/CodeGen/RDFRegisters.cpp
+++ b/llvm/lib/CodeGen/RDFRegisters.cpp
@@ -287,10 +287,8 @@ bool RegisterAggr::hasAliasOf(RegisterRef RR) const {
}
bool RegisterAggr::hasCoverOf(RegisterRef RR) const {
- if (RR.isMask()) {
- BitVector T(PRI.getMaskUnits(RR));
- return T.reset(Units).none();
- }
+ if (RR.isMask())
+ return PRI.getMaskUnits(RR).subsetOf(Units);
for (MCRegUnitMaskIterator U(RR.asMCReg(), &PRI.getTRI()); U.isValid(); ++U) {
auto [Unit, LaneMask] = *U;
diff --git a/llvm/tools/llvm-exegesis/lib/SnippetGenerator.cpp b/llvm/tools/llvm-exegesis/lib/SnippetGenerator.cpp
index 86d4e197b6063..b6c868c960828 100644
--- a/llvm/tools/llvm-exegesis/lib/SnippetGenerator.cpp
+++ b/llvm/tools/llvm-exegesis/lib/SnippetGenerator.cpp
@@ -274,11 +274,9 @@ static Error randomizeMCOperand(const LLVMState &State,
break;
case MCOI::OperandType::OPERAND_REGISTER: {
assert(Op.isReg());
- auto AllowedRegs = Op.getRegisterAliasing().sourceBits();
+ const BitVector &AllowedRegs = Op.getRegisterAliasing().sourceBits();
assert(AllowedRegs.size() == ForbiddenRegs.size());
- for (auto I : ForbiddenRegs.set_bits())
- AllowedRegs.reset(I);
- if (!AllowedRegs.any())
+ if (AllowedRegs.subsetOf(ForbiddenRegs))
return make_error<Failure>(
Twine("no available registers:\ncandidates:\n")
.concat(debugString(State.getRegInfo(),
|
|
@llvm/pr-subscribers-tools-llvm-exegesis Author: Anatoly Trosinenko (atrosinenko) ChangesReplace the code along these lines and with Full diff: https://github.com/llvm/llvm-project/pull/170876.diff 8 Files Affected:
diff --git a/bolt/include/bolt/Passes/LivenessAnalysis.h b/bolt/include/bolt/Passes/LivenessAnalysis.h
index 1df1113318d0b..f4faa1dc34ecd 100644
--- a/bolt/include/bolt/Passes/LivenessAnalysis.h
+++ b/bolt/include/bolt/Passes/LivenessAnalysis.h
@@ -37,10 +37,9 @@ class LivenessAnalysis : public DataflowAnalysis<LivenessAnalysis, BitVector,
virtual ~LivenessAnalysis();
bool isAlive(ProgramPoint PP, MCPhysReg Reg) const {
- BitVector BV = (*this->getStateAt(PP));
+ const BitVector &BV = *this->getStateAt(PP);
const BitVector &RegAliases = BC.MIB->getAliases(Reg);
- BV &= RegAliases;
- return BV.any();
+ return BV.anyCommon(RegAliases);
}
void run() { Parent::run(); }
diff --git a/bolt/include/bolt/Passes/ReachingDefOrUse.h b/bolt/include/bolt/Passes/ReachingDefOrUse.h
index 585d673e3b84e..41a6091aad4cb 100644
--- a/bolt/include/bolt/Passes/ReachingDefOrUse.h
+++ b/bolt/include/bolt/Passes/ReachingDefOrUse.h
@@ -133,8 +133,7 @@ class ReachingDefOrUse
RA.getInstClobberList(Point, Regs);
else
RA.getInstUsedRegsList(Point, Regs, false);
- Regs &= this->BC.MIB->getAliases(*TrackingReg);
- if (Regs.any())
+ if (Regs.anyCommon(this->BC.MIB->getAliases(*TrackingReg)))
Next.set(this->ExprToIdx[&Point]);
}
}
diff --git a/bolt/lib/Passes/RegReAssign.cpp b/bolt/lib/Passes/RegReAssign.cpp
index 0859cd244ce40..54eff51bfee68 100644
--- a/bolt/lib/Passes/RegReAssign.cpp
+++ b/bolt/lib/Passes/RegReAssign.cpp
@@ -316,18 +316,14 @@ void RegReAssign::aggressivePassOverFunction(BinaryFunction &Function) {
break;
}
- BitVector AnyAliasAlive = AliveAtStart;
- AnyAliasAlive &= BC.MIB->getAliases(ClassicReg);
- if (AnyAliasAlive.any()) {
+ if (AliveAtStart.anyCommon(BC.MIB->getAliases(ClassicReg))) {
LLVM_DEBUG(dbgs() << " Bailed on " << BC.MRI->getName(ClassicReg)
<< " with " << BC.MRI->getName(ExtReg)
<< " because classic reg is alive\n");
--End;
continue;
}
- AnyAliasAlive = AliveAtStart;
- AnyAliasAlive &= BC.MIB->getAliases(ExtReg);
- if (AnyAliasAlive.any()) {
+ if (AliveAtStart.anyCommon(BC.MIB->getAliases(ExtReg))) {
LLVM_DEBUG(dbgs() << " Bailed on " << BC.MRI->getName(ClassicReg)
<< " with " << BC.MRI->getName(ExtReg)
<< " because extended reg is alive\n");
diff --git a/bolt/lib/Passes/ShrinkWrapping.cpp b/bolt/lib/Passes/ShrinkWrapping.cpp
index fe342ccd38a67..b882e2512866d 100644
--- a/bolt/lib/Passes/ShrinkWrapping.cpp
+++ b/bolt/lib/Passes/ShrinkWrapping.cpp
@@ -1100,9 +1100,8 @@ SmallVector<ProgramPoint, 4> ShrinkWrapping::fixPopsPlacements(
bool Found = false;
if (SPT.getStateAt(ProgramPoint::getLastPointAt(*BB))->first ==
SaveOffset) {
- BitVector BV = *RI.getStateAt(ProgramPoint::getLastPointAt(*BB));
- BV &= UsesByReg[CSR];
- if (!BV.any()) {
+ const BitVector &BV = *RI.getStateAt(ProgramPoint::getLastPointAt(*BB));
+ if (!BV.anyCommon(UsesByReg[CSR])) {
Found = true;
PP = BB;
continue;
@@ -1110,9 +1109,8 @@ SmallVector<ProgramPoint, 4> ShrinkWrapping::fixPopsPlacements(
}
for (MCInst &Inst : llvm::reverse(*BB)) {
if (SPT.getStateBefore(Inst)->first == SaveOffset) {
- BitVector BV = *RI.getStateAt(Inst);
- BV &= UsesByReg[CSR];
- if (!BV.any()) {
+ const BitVector &BV = *RI.getStateAt(Inst);
+ if (!BV.anyCommon(UsesByReg[CSR])) {
Found = true;
PP = &Inst;
break;
diff --git a/bolt/lib/Passes/StackAvailableExpressions.cpp b/bolt/lib/Passes/StackAvailableExpressions.cpp
index a0d361f273de2..c685cc19badc3 100644
--- a/bolt/lib/Passes/StackAvailableExpressions.cpp
+++ b/bolt/lib/Passes/StackAvailableExpressions.cpp
@@ -103,8 +103,7 @@ bool StackAvailableExpressions::doesXKillsY(const MCInst *X, const MCInst *Y) {
else
RA.getInstClobberList(*Y, YClobbers);
- XClobbers &= YClobbers;
- return XClobbers.any();
+ return XClobbers.anyCommon(YClobbers);
}
BitVector StackAvailableExpressions::computeNext(const MCInst &Point,
diff --git a/bolt/lib/Passes/TailDuplication.cpp b/bolt/lib/Passes/TailDuplication.cpp
index 354f9b78830c3..e3ecf94a85070 100644
--- a/bolt/lib/Passes/TailDuplication.cpp
+++ b/bolt/lib/Passes/TailDuplication.cpp
@@ -97,8 +97,8 @@ bool TailDuplication::regIsPossiblyOverwritten(const MCInst &Inst, unsigned Reg,
getCallerSavedRegs(Inst, WrittenRegs, BC);
if (BC.MIB->isRep(Inst))
BC.MIB->getRepRegs(WrittenRegs);
- WrittenRegs &= BC.MIB->getAliases(Reg, false);
- return WrittenRegs.any();
+ const BitVector &AllAliases = BC.MIB->getAliases(Reg, false);
+ return WrittenRegs.anyCommon(AllAliases);
}
bool TailDuplication::regIsDefinitelyOverwritten(const MCInst &Inst,
@@ -117,8 +117,8 @@ bool TailDuplication::regIsUsed(const MCInst &Inst, unsigned Reg,
BinaryContext &BC) const {
BitVector SrcRegs = BitVector(BC.MRI->getNumRegs(), false);
BC.MIB->getSrcRegs(Inst, SrcRegs);
- SrcRegs &= BC.MIB->getAliases(Reg, true);
- return SrcRegs.any();
+ const BitVector &SmallerAliases = BC.MIB->getAliases(Reg, true);
+ return SrcRegs.anyCommon(SmallerAliases);
}
bool TailDuplication::isOverwrittenBeforeUsed(BinaryBasicBlock &StartBB,
diff --git a/llvm/lib/CodeGen/RDFRegisters.cpp b/llvm/lib/CodeGen/RDFRegisters.cpp
index 80fad722f592a..ee3e531c6fd5a 100644
--- a/llvm/lib/CodeGen/RDFRegisters.cpp
+++ b/llvm/lib/CodeGen/RDFRegisters.cpp
@@ -287,10 +287,8 @@ bool RegisterAggr::hasAliasOf(RegisterRef RR) const {
}
bool RegisterAggr::hasCoverOf(RegisterRef RR) const {
- if (RR.isMask()) {
- BitVector T(PRI.getMaskUnits(RR));
- return T.reset(Units).none();
- }
+ if (RR.isMask())
+ return PRI.getMaskUnits(RR).subsetOf(Units);
for (MCRegUnitMaskIterator U(RR.asMCReg(), &PRI.getTRI()); U.isValid(); ++U) {
auto [Unit, LaneMask] = *U;
diff --git a/llvm/tools/llvm-exegesis/lib/SnippetGenerator.cpp b/llvm/tools/llvm-exegesis/lib/SnippetGenerator.cpp
index 86d4e197b6063..b6c868c960828 100644
--- a/llvm/tools/llvm-exegesis/lib/SnippetGenerator.cpp
+++ b/llvm/tools/llvm-exegesis/lib/SnippetGenerator.cpp
@@ -274,11 +274,9 @@ static Error randomizeMCOperand(const LLVMState &State,
break;
case MCOI::OperandType::OPERAND_REGISTER: {
assert(Op.isReg());
- auto AllowedRegs = Op.getRegisterAliasing().sourceBits();
+ const BitVector &AllowedRegs = Op.getRegisterAliasing().sourceBits();
assert(AllowedRegs.size() == ForbiddenRegs.size());
- for (auto I : ForbiddenRegs.set_bits())
- AllowedRegs.reset(I);
- if (!AllowedRegs.any())
+ if (AllowedRegs.subsetOf(ForbiddenRegs))
return make_error<Failure>(
Twine("no available registers:\ncandidates:\n")
.concat(debugString(State.getRegInfo(),
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add unit tests? Wrong PR
52dbe76 to
24957f7
Compare
dcefe31 to
7c35ff5
Compare
24957f7 to
fb3eff2
Compare
fb3eff2 to
60e01b7
Compare
atrosinenko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will fix soon.
PS: Well, I cannot Approve my own PR, this is reasonable, but why I cannot Request changes? :)
| for (auto I : ForbiddenRegs.set_bits()) | ||
| AllowedRegs.reset(I); | ||
| if (!AllowedRegs.any()) | ||
| if (AllowedRegs.subsetOf(ForbiddenRegs)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, just spotted that the change in this line is incorrect due to AllowedRegs being later used as
AssignedValue = MCOperand::createReg(randomBit(AllowedRegs));There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted this particular change in b0f5356.
Replace the code along these lines
BitVector Tmp = LHS;
Tmp &= RHS;
return Tmp.any();
and
BitVector Tmp = LHS;
Tmp.reset(RHS);
return Tmp.none();
with `LHS.anyCommon(RHS)` and `LHS.subsetOf(RHS)`, correspondingly, which
do not require creating temporary BitVector and can return early.
60e01b7 to
2f79e84
Compare
atrosinenko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay, I rolled back the particular change to SnippetGenerator.cpp and checked once again that the conversions of BitVector BV = ... to const BitVector &BV = ... are legal and that there are no more reads from the bit vectors past the subsetOf or anyCommon calls.
I assume it should be safe for me to merge this as-is, as the final change is strictly a subset of the original one.
It still may make sense to simplify a couple places under llvm/tools/llvm-exegesis/ where iteration over set_bits() is immediately followed by reset(unsigned) calls, but it is probably reasonable to do in a separate PR (if at all), not to miss any subtle detail (such as would assert(LHS.size() == RHS.size()) still be required for anything else).
| for (auto I : ForbiddenRegs.set_bits()) | ||
| AllowedRegs.reset(I); | ||
| if (!AllowedRegs.any()) | ||
| if (AllowedRegs.subsetOf(ForbiddenRegs)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted this particular change in b0f5356.
…lvm#170876) Replace the code along these lines BitVector Tmp = LHS; Tmp &= RHS; return Tmp.any(); and BitVector Tmp = LHS; Tmp.reset(RHS); return Tmp.none(); with `LHS.anyCommon(RHS)` and `LHS.subsetOf(RHS)`, correspondingly, which do not require creating temporary BitVector and can return early.
| auto AllowedRegs = Op.getRegisterAliasing().sourceBits(); | ||
| assert(AllowedRegs.size() == ForbiddenRegs.size()); | ||
| if (AllowedRegs.subsetOf(ForbiddenRegs)) | ||
| for (auto I : ForbiddenRegs.set_bits()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: these types should be spelled out since they are not obvious based on the immediate context per https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
…lvm#170876) Replace the code along these lines BitVector Tmp = LHS; Tmp &= RHS; return Tmp.any(); and BitVector Tmp = LHS; Tmp.reset(RHS); return Tmp.none(); with `LHS.anyCommon(RHS)` and `LHS.subsetOf(RHS)`, correspondingly, which do not require creating temporary BitVector and can return early.

Replace the code along these lines
and
with
LHS.anyCommon(RHS)andLHS.subsetOf(RHS), correspondingly, whichdo not require creating temporary BitVector and can return early.